Conversation
This was referenced Mar 31, 2026
This was referenced Mar 31, 2026
Merged
Collaborator
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
5 tasks
eytan-starkware
approved these changes
Mar 31, 2026
Contributor
eytan-starkware
left a comment
There was a problem hiding this comment.
@eytan-starkware reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on TomerStarkware).
…nstead of panicking Fixes #9798
807d98d to
5248475
Compare
a5034b8 to
276b712
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Summary
Fixed a panic in the semantic analyzer when encountering mismatched generic parameter kinds between trait and impl functions by replacing
extract_matches!with proper error handling that skips diagnostics when the generic argument type doesn't match the expected type parameter.Type of change
Please check one:
Why is this change needed?
The semantic analyzer would panic when processing impl functions that had mismatched generic parameter kinds compared to their corresponding trait functions. Specifically, when a trait function expected a type parameter but the impl function provided an impl parameter (like
+Drop<felt252>), theextract_matches!macro would panic instead of gracefully handling the mismatch.What was the behavior or documentation before?
The code would panic with
extract_matches!when trying to extract aGenericArgumentId::Typefrom a generic argument that was actually a different kind (likeGenericArgumentId::Impl).What is the behavior or documentation after?
The code now properly handles the mismatch by checking the generic argument type with pattern matching and returning
skip_diagnostic()when the kinds don't match, allowing the previously reported diagnostic (WrongGenericParamKindForImplFunction) to be the primary error without causing a panic.Related issue or discussion (if any)
Fixes #9798
Additional context
The test case demonstrates the specific scenario: a trait function
bar<T>with a type parameterT, but an impl functionbar<+Drop<felt252>>with an impl parameter. The fix ensures that the appropriate error messages are shown (E2036 for parameter name incompatibility and E2038 for generic parameter kind incompatibility) without crashing the compiler.Note
Medium Risk
Touches core semantic substitution logic, which could affect how generics are rewritten and how errors are surfaced, but the change is narrowly scoped and guarded by an existing diagnostic path.
Overview
Fixes a crash in
SubstitutionRewriterwhen rewritingTypeLongId::GenericParameterby replacingextract_matches!with a kind check and returningskip_diagnostic()if the substitutedGenericArgumentIdisn’t aType.Adds a regression test covering a trait method
bar<T>implemented asbar<+Drop<felt252>>, asserting the expected E2036/E2038 diagnostics are emitted without panicking.Written by Cursor Bugbot for commit 276b712. This will update automatically on new commits. Configure here.